Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#175, #179, #135, #93, #183, #184, #191, fix broken html tests, fix broken --react-import-path #176

Open
wants to merge 16 commits into
base: gh-pages
Choose a base branch
from
Open

#175, #179, #135, #93, #183, #184, #191, fix broken html tests, fix broken --react-import-path #176

wants to merge 16 commits into from

Conversation

nippur72
Copy link
Contributor

This PR improves the parser for rt-scope as discussed in #175.

It's based on a regex, the improvement comes from scanning for as identifier; instead of just ; thus lowering the chances of picking a semicolon in the wrong place.

Of course there are still failing cases, but they are much less.

With this PR the following can be now parsed:

<div rt-scope="'as fast as possible' as message;';' as semicolon;'as' as as">
    {message}{semicolon}{as}
</div>

@nippur72
implements wix-incubator#175, better rt-scope syntax parser
6179e44
@nippur72
fixed broken html tests
f8296ce 
Fixed html tests that were not running due to changed file name.
Failing html tests now dump "actual" html instead of the code that
generates it (.actual.html)
Fixed title for rtStyle tests
Removed compareAndWriteHtml() that is no longer needed
@nippur72
improved rt-scope syntax parser
883a10b 
rt-scope syntax parser now is able to scan over quoted strings (both
single and double quotes) which may be also contain escaped quotes (\"
or \')
@nippur72 nippur72 changed the title implements #175, better rt-scope syntax parser better rt-scope syntax parser #175, fix broken html tests Aug 2, 2016
@nippur72
Copy link
Contributor Author

nippur72 commented Aug 2, 2016

With the latest commits:

  • improved the regex used in the parser that now scans over quoted strings (both
    single and double quotes) taking into account escaped quotes (\" or \'). Now there should be no more failing cases for the rt-scope syntax (e.g. rt-scope="'x as id;' as id;" does compile correctly).
  • fixed html tests that were not running after they were split from rt.valid.spec.js into rt-html-valid.js.

@nippur72 nippur72 changed the title better rt-scope syntax parser #175, fix broken html tests better rt-scope syntax parser #175, #179 + fix broken html tests Aug 3, 2016
@nippur72 nippur72 mentioned this pull request Aug 3, 2016
Closed
@nippur72 nippur72 changed the title better rt-scope syntax parser #175, #179 + fix broken html tests better rt-scope syntax parser #175, #179, #135, fix broken html tests Aug 12, 2016
@nippur72
Copy link
Contributor Author

This PR now implements also #135 with the new (optional) CLI option --create-element-alias.

If an alias is specified, it will be used in place of React.createElement() thus making shorter calls and helping the minifier.

Example of rt foo.rt --create-element-alias h

'use strict';
var h = require('react').createElement;
var _ = require('lodash');
module.exports = function () {
    return h('div', {}, h('span', {}, 'Hello'));
};

@nippur72 nippur72 mentioned this pull request Aug 12, 2016
Open
@nippur72 nippur72 changed the title better rt-scope syntax parser #175, #179, #135, fix broken html tests #175, #179, #135, fix broken html tests, fix broken --react-import-path Aug 15, 2016
@nippur72
Copy link
Contributor Author

This PR now fixes also an issue with -react-import-path when the path is different from react.

Regarding the --create-element-alias, now is possible to have a different function name than createElement (e.g. if using a different library than React like preact). Just add it after a comma:

rt foo.rt --react-import-path preact --create-element-alias h,h

will generate:

var h = require('preact').h;
module.exports = function () {
    return h('div', {}, h('span', {}, 'Hello'));
};

@nippur72 nippur72 mentioned this pull request Aug 15, 2016
Open
@nippur72 nippur72 mentioned this pull request Aug 19, 2016
Open
@nippur72 nippur72 changed the title #175, #179, #135, fix broken html tests, fix broken --react-import-path #175, #179, #135, #93, #183, #184, fix broken html tests, fix broken --react-import-path Aug 19, 2016
@nippur72
Copy link
Contributor Author

this PR now implements also #93, #183 and #184 (see for discussion).

@nippur72
Copy link
Contributor Author

nippur72 commented Sep 5, 2016

this PR now fixes also #190

@nippur72
Copy link
Contributor Author

nippur72 commented Sep 6, 2016

...and #191

@nippur72 nippur72 changed the title #175, #179, #135, #93, #183, #184, fix broken html tests, fix broken --react-import-path #175, #179, #135, #93, #183, #184, #191, fix broken html tests, fix broken --react-import-path Sep 6, 2016
@nippur72 nippur72 mentioned this pull request Sep 22, 2016
Open
@nippur72 nippur72 mentioned this pull request Nov 4, 2017
Open
@idok
Copy link
Contributor

idok commented Nov 12, 2017

Hi nino,
Can you split this PR so we can try and merge at least some of it?

@nippur72
Copy link
Contributor Author

sure, I will split it into as small as possible PRs, though it might take a while since I have to do some reconciliation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nippur72 @idok